electron/ipc/cursor/interaction.ts (1)

303-339: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Consider updating lastKeystrokeTimeMs on collapsed events for complete auto-repeat suppression.

lastKeystrokeTimeMs is only updated when a keystroke is recorded, not when it's collapsed. This means a held key with 30ms auto-repeat intervals will record one event every ~45ms (every other repeat passes the threshold). If complete suppression is the intent, update lastKeystrokeTimeMs on every event so subsequent repeats are always within the 45ms window of the last seen event.

If partial collapsing is intentional (e.g., to show held-key state in the overlay), disregard.

♻️ Optional: complete auto-repeat suppression

 			// Collapse auto-repeat: ignore the same physical key re-firing rapidly.
 			if (keycode === lastKeystrokeCode && timeMs - lastKeystrokeTimeMs < 45) {
+				lastKeystrokeTimeMs = timeMs;
 				return;
 			}
 			lastKeystrokeCode = keycode;
 			lastKeystrokeTimeMs = timeMs;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/ipc/cursor/interaction.ts` around lines 303 - 339, The `onKeyDown`
auto-repeat collapse logic in `interaction.ts` only updates
`lastKeystrokeTimeMs` when a keystroke is recorded, so repeated keydowns can
still leak through every ~45ms. Update the state inside `onKeyDown` so
`lastKeystrokeTimeMs` (and, if needed, `lastKeystrokeCode`) reflects every seen
keydown event, including collapsed ones, while keeping the existing privacy gate
and `pushKeystrokeSample` behavior intact.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/components/video-editor/videoPlayback/keystrokeOverlay/keystrokeCoalescing.ts (2)

86-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant open && guard.

canExtend already checks open && on line 88, so the open && on line 93 is redundant. This is harmless but slightly reduces readability — a reader might wonder if the extra check guards against a different condition.

♻️ Proposed refactor

-		if (open && canExtend) {
+		if (canExtend) {

Since canExtend is open && ..., it is falsy when open is undefined, so the behavior is identical.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/components/video-editor/videoPlayback/keystrokeOverlay/keystrokeCoalescing.ts`
around lines 86 - 93, Remove the redundant open guard from the conditional in
the keystroke coalescing logic: since canExtend already includes the open check,
use canExtend alone in the if statement while preserving all existing extension
behavior.

---

29-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Potential groupId collision on duplicate events.

If two events share the same timeMs, key, and modifier state (e.g., a genuinely repeated keypress at the same timestamp, or a data pipeline producing duplicate samples), they will produce identical ids. Since both could start separate groups (if they fall outside groupWindowMs of each other but within the lookback window), the downstream renderer would encounter duplicate React keys.

Consider appending a monotonic index from the relevant array to guarantee uniqueness while remaining position-independent relative to the sliding window:

♻️ Proposed refactor

 function groupId(event: KeystrokeEvent): string {
 	const mods =
 		`${event.ctrl ? "c" : ""}${event.alt ? "a" : ""}` +
 		`${event.shift ? "s" : ""}${event.meta ? "m" : ""}`;
-	return `k${event.timeMs}:${event.key}:${mods}`;
+	return `k${event.timeMs}:${event.key}:${mods}`;
 }

Alternatively, pass an index into groupId from the relevant array iteration:

-	for (const event of relevant) {
+	for (let i = 0; i < relevant.length; i++) {
+		const event = relevant[i];
 		// ...
 		if (isChord) {
 			groups.push({
-				id: groupId(event),
+				id: groupId(event, i),

-function groupId(event: KeystrokeEvent): string {
+function groupId(event: KeystrokeEvent, index: number): string {
 	const mods =
 		`${event.ctrl ? "c" : ""}${event.alt ? "a" : ""}` +
 		`${event.shift ? "s" : ""}${event.meta ? "m" : ""}`;
-	return `k${event.timeMs}:${event.key}:${mods}`;
+	return `k${event.timeMs}:${event.key}:${mods}:${index}`;
 }

This keeps the id stable across frames (same relevant ordering) while guaranteeing uniqueness.

The index from relevant is stable across frames for the same event because relevant is sorted by timeMs — the same event always occupies the same position among events with identical timestamps. This is a low-risk edge case but worth hardening if duplicate timestamps are possible upstream.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/components/video-editor/videoPlayback/keystrokeOverlay/keystrokeCoalescing.ts`
around lines 29 - 34, Update groupId and its relevant-array call site to
incorporate the event’s monotonic index from the relevant iteration, ensuring
duplicate events with identical timeMs, key, and modifiers receive distinct IDs.
Preserve the existing ID components and use the stable relevant ordering so IDs
remain consistent across frames.

src/components/video-editor/projectPersistence.ts (1)

109-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid re-declaring the keystroke position union inline.

keystrokePosition re-declares the same literal union already exported as KeystrokeOverlayPosition from keystrokeOverlay/keystrokeTypes.ts. Importing and reusing that type avoids silent drift if a position value is ever added/removed.

♻️ Suggested fix

+import type { KeystrokeOverlayPosition } from "./videoPlayback/keystrokeOverlay/keystrokeTypes";
...
-	keystrokePosition: "bottom-center" | "bottom-left" | "bottom-right" | "top-center";
+	keystrokePosition: KeystrokeOverlayPosition;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/video-editor/projectPersistence.ts` around lines 109 - 111,
Update the project persistence type’s keystrokePosition field to reuse the
exported KeystrokeOverlayPosition type from keystrokeOverlay/keystrokeTypes.ts,
adding the necessary type import and removing the inline literal union.

src/components/video-editor/VideoEditor.tsx (1)

765-769: 🎯 Functional Correctness | 🔵 Trivial

Placeholder keystroke values in preset snapshot — already tracked via TODO.

showKeystrokes/keystrokePosition/keystrokeSize are hardcoded rather than using the real state defined earlier in this same diff (lines 474-478). The TODO already documents this is deferred until presets manage these settings — just flagging for visibility once that follow-up lands, since applyEditorPresetSnapshot (923-979) correspondingly never restores these fields either.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/video-editor/VideoEditor.tsx` around lines 765 - 769, Keep the
current placeholder keystroke values and TODO in the preset snapshot for now; no
immediate change is required. When preset support is implemented, update the
snapshot construction near the keystroke state and applyEditorPresetSnapshot to
save and restore showKeystrokes, keystrokePosition, and keystrokeSize using the
corresponding VideoEditor state.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes